Support indefinite length BER encodings in PKCS #7 #1380
Conversation
SparkiDev
force-pushed
the
ber_indef
branch
2 times, most recently
from
53e6812 to
5158da6
Compare
wolfcrypt/src/asn.c
Outdated
There was a problem hiding this comment.
Since this is a public function we should have a length input for Der size as well so we don't overwrite the Der buffer. It will also catch programming errors on our side.
wolfcrypt/src/pkcs7.c
Outdated
There was a problem hiding this comment.
We should check the return code here, there's no guarantee in the future that if the first call succeeds on the length check that the second call will succeed as well. Also a good static analyzer should warn on this.
wolfcrypt/src/pkcs7.c
Outdated
There was a problem hiding this comment.
return code check needed.
|
Looks good, please resolve conflict. |
dgarske
left a comment
dgarske
left a comment
There was a problem hiding this comment.
Do we have a BER indefinite length encoded message we can add a test with? It would be ideal to have a wolfCrypt test for wc_BerToDer or for the PKCS7 calls.
wolfcrypt/src/pkcs7.c
Outdated
| ret = wc_BerToDer(pkiMsg, pkiMsgSz, NULL, &len); | ||
| if (ret != LENGTH_ONLY_E) | ||
| return ret; | ||
| pkcs7->der = XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); |
There was a problem hiding this comment.
We find results from XMALLOC need to have a cast in front due to some compilers. (byte*)XMALLOC().
wolfcrypt/test/test.c
Outdated
| pkcs7.unprotectedAttribs = testVectors[i].attribs; | ||
| pkcs7.unprotectedAttribsSz = testVectors[i].attribsSz; | ||
| pkcs7.heap = HEAP_HINT; | ||
| #ifdef ASN_BER_TO_DER |
There was a problem hiding this comment.
Recommend doing an wc_PKCS7_Init above and remove this. Our test examples should do the init, so the PKCS7 struct is memset to 0.
|
@dgarske sent an example case by email just in case we still need one. |
dgarske
left a comment
dgarske
left a comment
There was a problem hiding this comment.
The changes look great! I'm marking this approved. Up to you if you want to add a BER to DER test in this PR or if you want to add later.
4 participants
Also support explicit octet string in enveloped data.